You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before the X509CertificateLoader was merged, the ReadECDsaPrivateKey_BrainpoolP160r1_Pfx test on Android was passing, but for the wrong reason. The test is asserting that the platform throws a CryptographicException for brainpool curves. On Android it was throwing a CryptographicException because the PFX is protected with RC2, which is unavailable on Android.
This PR changes the PBE decryption to wrap the PlatformNotSupportedException with a CryptographicException with the same message, restoring the previous behavior.
We should probably investigate fixing the brainpool test to use an algorithm other than RC2 so we can confirm the Android tests fail for the right reason, but this unblocks CI.
Before loader merge:
Exception messages: System.Security.Cryptography.CryptographicException : The certificate data cannot be read with the provided password, the password may be incorrect.
---- System.PlatformNotSupportedException : Algorithm 'RC2' is not supported on this platform. Exception stack traces: at System.Security.Cryptography.X509Certificates.UnixPkcs12Reader.Decrypt(SafePasswordHandle password, Boolean ephemeralSpecified)
at System.Security.Cryptography.X509Certificates.AndroidCertificatePal.ReadPkcs12(ReadOnlySpan`1 rawData, SafePasswordHandle password, Boolean ephemeralSpecified)
at System.Security.Cryptography.X509Certificates.AndroidCertificatePal.FromBlob(ReadOnlySpan`1 rawData, SafePasswordHandle password, Boolean readingFromFile, X509KeyStorageFlags keyStorageFlags)
at System.Security.Cryptography.X509Certificates.AndroidCertificatePal.FromBlob(ReadOnlySpan`1 rawData, SafePasswordHandle password, X509KeyStorageFlags keyStorageFlags)
at System.Security.Cryptography.X509Certificates.CertificatePal.FromBlob(ReadOnlySpan`1 rawData, SafePasswordHandle password, X509KeyStorageFlags keyStorageFlags)
at System.Security.Cryptography.X509Certificates.X509Certificate..ctor(Byte[] rawData, String password, X509KeyStorageFlags keyStorageFlags)
at System.Security.Cryptography.X509Certificates.X509Certificate..ctor(Byte[] rawData, String password)
at System.Security.Cryptography.X509Certificates.X509Certificate2..ctor(Byte[] rawData, String password)
After this pull request:
Exception messages: System.Security.Cryptography.CryptographicException : The certificate data cannot be read with the provided password, the password may be incorrect.
---- System.PlatformNotSupportedException : Algorithm 'RC2' is not supported on this platform. Exception stack traces: at System.Security.Cryptography.X509Certificates.X509CertificateLoader.ThrowWithHResult(String message, Int32 hResult, Exception innerException)
at System.Security.Cryptography.X509Certificates.X509CertificateLoader.BagState.DecryptSafeContents(AlgorithmIdentifierAsn& algorithmIdentifier, ReadOnlySpan`1 passwordSpan, ReadOnlySpan`1 encryptedContent)
at System.Security.Cryptography.X509Certificates.X509CertificateLoader.DecryptSafeContents(ContentInfoAsn safeContentsAsn, Pkcs12LoaderLimits loaderLimits, ReadOnlySpan`1 passwordSpan, BagState& bagState, Nullable`1& workRemaining)
at System.Security.Cryptography.X509Certificates.X509CertificateLoader.ReadCertsAndKeys(BagState& bagState, ReadOnlyMemory`1 data, ReadOnlySpan`1& password, Pkcs12LoaderLimits loaderLimits)
at System.Security.Cryptography.X509Certificates.X509CertificateLoader.LoadPkcs12(ReadOnlyMemory`1 data, ReadOnlySpan`1 password, X509KeyStorageFlags keyStorageFlags, Pkcs12LoaderLimits loaderLimits)
at System.Security.Cryptography.X509Certificates.X509CertificateLoader.LoadPkcs12Pal(ReadOnlySpan`1 data, ReadOnlySpan`1 password, X509KeyStorageFlags keyStorageFlags, Pkcs12LoaderLimits loaderLimits)
at System.Security.Cryptography.X509Certificates.AndroidCertificatePal.FromBlob(ReadOnlySpan`1 rawData, SafePasswordHandle password, Boolean readingFromFile, X509KeyStorageFlags keyStorageFlags)
at System.Security.Cryptography.X509Certificates.AndroidCertificatePal.FromBlob(ReadOnlySpan`1 rawData, SafePasswordHandle password, X509KeyStorageFlags keyStorageFlags)
at System.Security.Cryptography.X509Certificates.CertificatePal.FromBlob(ReadOnlySpan`1 rawData, SafePasswordHandle password, X509KeyStorageFlags keyStorageFlags)
at System.Security.Cryptography.X509Certificates.X509Certificate..ctor(Byte[] rawData, String password, X509KeyStorageFlags keyStorageFlags)
at System.Security.Cryptography.X509Certificates.X509Certificate..ctor(Byte[] rawData, String password)
at System.Security.Cryptography.X509Certificates.X509Certificate2..ctor(Byte[] rawData, String password)
This PR changes the PBE decryption to wrap the PlatformNotSupportedException with a CryptographicException with the same message, restoring the previous behavior.
The test is actually checking the exception message? We generally try not to do that.
The test is actually checking the exception message? We generally try not to do that.
No, the test does not do that. When I say "same message" meant the product behavior. "Let's go back to throwing a CryptographicException like we were before the loader was merged, and the exception text that I used for the CryptographicException is also the same as it was before".
The test is actually checking the exception message?
I don't think it is, since in the premise he points out that me regressing CryptographicException to PNSE here unmasked that we have a test that is passing, but for the wrong reason.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before the X509CertificateLoader was merged, the
ReadECDsaPrivateKey_BrainpoolP160r1_Pfxtest on Android was passing, but for the wrong reason. The test is asserting that the platform throws a CryptographicException for brainpool curves. On Android it was throwing a CryptographicException because the PFX is protected with RC2, which is unavailable on Android.This PR changes the PBE decryption to wrap the PlatformNotSupportedException with a CryptographicException with the same message, restoring the previous behavior.
We should probably investigate fixing the brainpool test to use an algorithm other than RC2 so we can confirm the Android tests fail for the right reason, but this unblocks CI.
Before loader merge:
After this pull request:
Fixes #104030